Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce cider-shorten-error-overlays customization option #3531

Merged
merged 2 commits into from
Oct 17, 2023
Merged

Conversation

vemv
Copy link
Member

@vemv vemv commented Oct 16, 2023

Fixes #3525

@vemv vemv requested a review from bbatsov October 16, 2023 20:17
@bbatsov
Copy link
Member

bbatsov commented Oct 16, 2023

Seems to me we don't really need a defcustom for this. Given that the offending line would already be underlined this should simply be the only behavior. There's no need to make everything customizable. :-)

@vemv
Copy link
Member Author

vemv commented Oct 16, 2023

I tend to agree, but #3338 (reply in thread) showed a reasonable case for the alternative.

@bbatsov
Copy link
Member

bbatsov commented Oct 16, 2023

Hmm, it seems to me we're solving the wrong problem here - if the highlighting is not precise/useful probably we should improve it. Obviously adding this extra defcustom is not a big deal for us, but it just seems weird to me to have such a high granularity of customizations, that probably almost no one will ever touch.

@vemv
Copy link
Member Author

vemv commented Oct 16, 2023

My take would be that different Clojure developers need/want different degrees of detail, for errors.

Personally, I feel that I have seen certain errors so many times, that just a short string will suffice to inform me.

But not everyone operates like that.

@bbatsov
Copy link
Member

bbatsov commented Oct 17, 2023

My take would be that different Clojure developers need/want different degrees of detail, for errors.

This part I get, but I believe that level of granularity is something like a line in an overlay vs a stacktrace buffer. Now we're discussing whether we need a defcustom for a message. Every such branch/defcustom adds some maintenance complexity, that's why it's good to consider carefully their value and their alternatives.

cider-eval.el Outdated
and the suffix matched by `cider-module-info-regexp' will be removed."
:type 'boolean
:group 'cider
:package-version '(cider . "0.19.0"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, this should be 1.9, not 0.19. :-)

@vemv
Copy link
Member Author

vemv commented Oct 17, 2023

Maybe a middle ground solution would be to make cider--shorten-error-message public such that users can advice it to act like the identity function.

cider-eval.el Outdated
@@ -821,6 +821,24 @@ REPL buffer. This is controlled via
conn)))
(nrepl-dict-get result "phase"))))))

(defcustom cider-shorten-error-overlays t
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd name this cider-shorten-inline-errors or cider-shorten-inline-error-messages. I think that the overlay is an implementation detail, that might change in the future, so it's better to focus on the essence (that we should an error message inline).

@@ -207,6 +207,15 @@ bottom) with the `cider-use-overlays` variable:
(setq cider-use-overlays nil)
----

Overlays that indicate errors are by default trimmed of file/line/phase information.
If you find it useful, you can customize:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to also put two error messages side by side here, so it clearer to people how the messages look in both cases.

@bbatsov
Copy link
Member

bbatsov commented Oct 17, 2023

Maybe a middle ground solution would be to make cider--shorten-error-message public such that users can advice it to act like the identity function.

Hmm, actually let's make a defcustom which is the function used to process the error message. We can provide the current function as a default for it, and people can set it to nil so it'd just be the raw message without any processing. We have this in other places of CIDER as well - e.g. that's how the REPL prompt can be customized.

@vemv vemv merged commit 27ed547 into master Oct 17, 2023
26 checks passed
@vemv vemv deleted the 3525 branch October 17, 2023 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the error overlay trimming optional
2 participants